src/Consolonia.Core/Drawing/BitmapImpl.cs (2)

42-42: Document the height scaling factor

The height scaling factor of 0.55 in PixelSize seems arbitrary. Consider adding a documentation comment explaining why this specific scaling factor is used and its implications.

---

40-40: Consider making DPI configurable

The DPI value is hardcoded to 96, which might not be suitable for all display scenarios. Consider making this configurable or retrieving it from the system settings.

src/Consolonia.Core/Drawing/ConsoloniaRenderInterface.cs (1)

54-54: Remove Unnecessary Commented-Out Code

The commented-out line at 54 is unnecessary and can be removed to improve code readability.

Apply this diff to remove the commented code:

-// return new ConsoleRenderTargetBitmapImpl(size, dpi);

src/Consolonia.Core/Drawing/DrawingContextImpl.cs (4)

141-153: Improve code readability by formatting nested loops and blocks

The nested for loops starting at line 141 lack proper indentation, which affects code readability and maintainability.

Ensure that all code blocks are properly indented to enhance clarity:

for (int x = 0; x < width; x++)
{
    for (int y = 0; y < height; y++)
    {
        // Code inside inner loop
    }
}

🧰 Tools

🪛 GitHub Check: build

[notice] 151-151:
"[ConvertToLambdaExpression] Use lambda expression" on /home/runner/work/Consolonia/Consolonia/src/Consolonia.Core/Drawing/DrawingContextImpl.cs(151,46)

---

Line range hint 270-289: Handle unexpected Brush types in ExtractColorOrNullWithPlatformCheck

The method assumes that pen.Brush is of specific types. If an unexpected Brush type is passed, it may result in an exception or unintended behavior.

Consider adding a default case or throwing a more descriptive exception to handle unexpected Brush types:

switch (pen.Brush)
{
    case LineBrush lineBrush:
        lineStyle = lineBrush.LineStyle;
        break;
    case ConsoleBrush consoleBrush:
        // Existing handling
        break;
    case ImmutableSolidColorBrush solidColorBrush:
        // Existing handling
        break;
    default:
        throw new NotSupportedException($"Brush type '{pen.Brush.GetType()}' is not supported.");
}

---

Line range hint 360-378: Make tabSize a configurable constant

In the DrawStringInternal method, tabSize is hardcoded to 8. For better flexibility and maintainability, define tabSize as a constant or make it configurable.

Apply this diff to define tabSize as a private constant:

+private const int TabSize = 8;

...

case '\t':
{
-    const int tabSize = 8;
     var consolePixel = new Pixel(' ', foregroundColor);
-    for (int j = 0; j < tabSize; j++)
+    for (int j = 0; j < TabSize; j++)
     {
         Point newCharacterPoint = characterPoint.WithX(characterPoint.X + j);
         CurrentClip.ExecuteWithClipping(newCharacterPoint, () =>
         {
             _pixelBuffer.Set((PixelBufferCoordinate)newCharacterPoint,
                 (oldPixel, cp) => oldPixel.Blend(cp), consolePixel);
         });
     }

-    currentXPosition += tabSize - 1;
+    currentXPosition += TabSize - 1;
}

---

151-151: Simplify lambda expression using method group

Static analysis suggests converting the lambda expression to a method group for simplicity.

Apply this diff to simplify the lambda expression:

_pixelBuffer.Set(new PixelBufferCoordinate((ushort)localPx, (ushort)localPy),
-    (existingPixel, _) => existingPixel.Blend(imagePixel), imagePixel.Background.Color);
+    existingPixel => existingPixel.Blend(imagePixel), imagePixel.Background.Color);

🧰 Tools

🪛 GitHub Check: build

[notice] 151-151:
"[ConvertToLambdaExpression] Use lambda expression" on /home/runner/work/Consolonia/Consolonia/src/Consolonia.Core/Drawing/DrawingContextImpl.cs(151,46)

Configuration used: CodeRabbit UI
Review profile: CHILL

src/Consolonia.Core/Drawing/DrawingContextImpl.cs (1)

51-58: Ensure proper memory management with SkiaSharp resources

The bitmap drawing implementation should dispose of all SkiaSharp resources properly. While using statements are correctly applied for bitmap and canvas, consider wrapping the entire operation in a try-finally block to ensure cleanup in case of exceptions.

Apply this diff:

 public void DrawBitmap(IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect)
 {
+    try
+    {
         var targetRect = new Rect(Transform.Transform(new Point(destRect.TopLeft.X, destRect.TopLeft.Y)),
             Transform.Transform(new Point(destRect.BottomRight.X, destRect.BottomRight.Y)));
         var bmp = (BitmapImpl)source;
         using var bitmap = new SKBitmap((int)targetRect.Width, (int)targetRect.Height);
         using var canvas = new SKCanvas(bitmap);
         canvas.DrawBitmap(bmp.Bitmap, new SKRect(0, 0, (float)targetRect.Width, (float)targetRect.Height), 
             new SKPaint { FilterQuality = SKFilterQuality.Medium });
+    }
+    finally
+    {
+        // Ensure any additional resources are cleaned up
+    }

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Search for clipping implementations in the codebase
rg -l "Intersect.*clip" --type cs

Configuration used: CodeRabbit UI
Review profile: CHILL

-//if (!Transform.IsTranslateOnly()) ConsoloniaPlatform.RaiseNotSupported(15);
+// TODO: Document why non-translate transforms are not supported
+if (!Transform.IsTranslateOnly())
+    throw new NotSupportedException("Only translate transforms are supported for text rendering");

-for (int x = 0; x < width; x++)
-for (int y = 0; y < height; y++)
+Parallel.For(0, width, x =>
+{
+    for (int y = 0; y < height; y++)
     {
         int px = (int)targetRect.TopLeft.X + x;
         int py = (int)targetRect.TopLeft.Y + y;
         SKColor skColor = bitmap.GetPixel(x, y);
         Color color = Color.FromRgb(skColor.Red, skColor.Green, skColor.Blue);
         var imagePixel = new Pixel('█', color);
         CurrentClip.ExecuteWithClipping(new Point(px, py),
             () =>
             {
                 _pixelBuffer.Set(new PixelBufferCoordinate((ushort)px, (ushort)py),
                     (existingPixel, _) => existingPixel.Blend(imagePixel), imagePixel.Background.Color);
             });
     }
+});

src/Consolonia.Core/Drawing/BitmapImpl.cs (1)

40-40: Document the purpose of the 0.55 scaling factor.

The hardcoded scaling factor of 0.55 for height seems arbitrary. Consider:

1. Adding a comment explaining its purpose
2. Making it configurable if it's related to console character aspect ratio

Configuration used: CodeRabbit UI
Review profile: CHILL

src/Consolonia.Core/Drawing/BitmapImpl.cs (2)

2-2: Remove unused using directive.

The System.Globalization namespace is not used in the current implementation.

-using System.Globalization;

🧰 Tools

🪛 GitHub Check: build

[warning] 2-2:
"[RedundantUsingDirective] Using directive is not required by the code and can be safely removed" on /home/runner/work/Consolonia/Consolonia/src/Consolonia.Core/Drawing/BitmapImpl.cs(2,1)

---

38-38: Consider making DPI configurable.

The DPI is hardcoded to 96x96, which might not be suitable for all display scenarios. Consider making it configurable or detecting it from the system.

Configuration used: CodeRabbit UI
Review profile: CHILL